Skip to content

Conversation

3reality-support
Copy link

@3reality-support 3reality-support commented Jun 6, 2025

Proposed change

This PR added adaptation code for dual_plug and added 2 additional private clusters

  1. reset_summation_delivered---reset the accumulated power of the plug
  2. on_to_off_delay---off delay, when you set the attribute to 10 and the current status is on, it will change to off after 10 seconds
  3. off_to_on_delay---on delay, similar to the above
  4. If both on_to_off_delay and off_to_of_delay are set, the plug will loop on and off

Additional information

2ca2370e55bad991f254209e0406c61
a0886f3197e45043ea09cbb4b9e4023
b15b302acac803df5e1e733339e9b99

Checklist

  • The changes are tested and work correctly
  • pre-commit checks pass / the code has been formatted using Black
  • Tests have been added to verify that the new code works

jintj and others added 30 commits August 30, 2024 10:05
@3reality-support
Copy link
Author

@puddly @TheJulianJES Please help to check

@3reality-support
Copy link
Author

@TheJulianJES

@TheJulianJES TheJulianJES changed the title Add Third Reality dual_plug and modify plug private cluster Add Third Reality dual plug settings Jul 12, 2025
@TheJulianJES TheJulianJES changed the title Add Third Reality dual plug settings Add Third Reality dual plug and single plug settings Jul 12, 2025
Comment on lines 74 to 89
translation_key="on_to_off_delay_ep1",
fallback_name="Turn off delay",
)
.number(
attribute_name=ThirdRealityPlugCluster.AttributeDefs.on_to_off_delay.name,
cluster_id=ThirdRealityPlugCluster.cluster_id,
endpoint_id=2,
min_value=0,
max_value=65535,
step=1,
mode="box",
unit=UnitOfTime.SECONDS,
device_class=NumberDeviceClass.DURATION,
translation_key="on_to_off_delay_ep2",
fallback_name="Turn off delay",
)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fallback_name will be used as the English translation for the translation_key when this makes its way into HA. Here, the translation keys are different for the two endpoints, but the fallback name is the same.
We should just change the fallback_name to either be "Turn off delay 1" and "Turn off delay 2" or be something like "Turn off delay left" and "Turn off delay right".

In the future, we'll try and add a postfix if there are entities named in the same way on multiple endpoints. For now, it's fine to explicitly make the translation key and fallback name different. Other integrations in HA also do this.

Please also adjust all other occurrences.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mean that the translation_key for ep1 and ep2 can be the same.But there was an error on my end
3129505c582599ab562f5ca2761bc44

Comment on lines 64 to 89
.number(
attribute_name=ThirdRealityPlugCluster.AttributeDefs.on_to_off_delay.name,
cluster_id=ThirdRealityPlugCluster.cluster_id,
endpoint_id=1,
min_value=0,
max_value=65535,
step=1,
mode="box",
unit=UnitOfTime.SECONDS,
device_class=NumberDeviceClass.DURATION,
translation_key="on_to_off_delay_ep1",
fallback_name="Turn off delay",
)
.number(
attribute_name=ThirdRealityPlugCluster.AttributeDefs.on_to_off_delay.name,
cluster_id=ThirdRealityPlugCluster.cluster_id,
endpoint_id=2,
min_value=0,
max_value=65535,
step=1,
mode="box",
unit=UnitOfTime.SECONDS,
device_class=NumberDeviceClass.DURATION,
translation_key="on_to_off_delay_ep2",
fallback_name="Turn off delay",
)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, have you checked that the two entities each for the dual plug both show up in HA?
I thought the unique IDs might conflict, as they're just based on the attribute name, but we might also include the endpoint. I'd have to check.

If both appear, this is fine. Otherwise, you can include unique_id_suffix="on_to_off_delay_1" (and _2) for the entities to assign a different UID.


(
QuirkBuilder("Third Reality, Inc", "3RDP01072Z")
.also_applies_to("Third Reality, Inc", "3RWP01073Z")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's use applies_to. Both do the same, but we've switched to just using applies_to in most quirks.

min_value=0,
max_value=65535,
step=1,
mode="box",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mode=box is fine to keep, but we don't actually set that mode in ZHA yet.
So, it's expected that this does nothing (for now). There's an open issue in the ZHA repo about this.

cluster_id=ThirdRealityPlugCluster.cluster_id,
endpoint_id=2,
translation_key="reset_summation_delivered_ep2",
fallback_name="Reset right summation delivered ep2", # ep2 is right
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is similar to my previous comment, let's just include either left/right or 1/2, not both and not something abbreviated like "ep2".

Copy link
Collaborator

@TheJulianJES TheJulianJES left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Otherwise, I think this looks good. We'll come back to this when the last few things are addressed.
Lastly, there's no reason to ping us every couple of days. We'll try to review PRs as soon as possible.

@TheJulianJES TheJulianJES added the smash This PR is close to be merged soon label Jul 12, 2025
@3reality-support
Copy link
Author

3reality-support commented Jul 14, 2025

Otherwise, I think this looks good. We'll come back to this when the last few things are addressed. Lastly, there's no reason to ping us every couple of days. We'll try to review PRs as soon as possible.

Sorry, I got it. Also, do I need to @ you every time I make modifications. Or quote your comment

@3reality-support
Copy link
Author

@TheJulianJES

@TheJulianJES TheJulianJES added the needs reviewer answer An answer from a reviewer is needed (e.g. why a PR isn't acceptable in the current state). label Jul 30, 2025
@3reality-support
Copy link
Author

@TheJulianJES

@TheJulianJES TheJulianJES added the manufacturer This request was made by the device's manufacturer label Sep 2, 2025
Copy link
Collaborator

@TheJulianJES TheJulianJES left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'll likely automatically add a postfix number to all entities named the same in the future: zigpy/zha#525

I'll come back to this shortly. For clarity, you can still use different translation keys with left and right though. Those won't be impacted by the change then, since they'll be named differently.

attribute_value=0x01, # 1 reset summation delivered
cluster_id=ThirdRealityPlugCluster.cluster_id,
endpoint_id=1,
translation_key="reset_summation_delivered1",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add an underscore before the 1 at the end. Do this for all translation keys.

In this case, it might make even more sense to rename the translation_key to reset_summation_delivered_left, since that's what the fallback_name (translation value) will be. If the translation were to include 1 or 2, we should mention that in the translation key. But if it's left or right, we should instead mention that.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK. Modified, please check

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
manufacturer This request was made by the device's manufacturer needs reviewer answer An answer from a reviewer is needed (e.g. why a PR isn't acceptable in the current state). smash This PR is close to be merged soon
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants